Skip to content

feat: Tool timeout + recover from hanging #5515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 13 commits into from
Closed

feat: Tool timeout + recover from hanging #5515

wants to merge 13 commits into from

Conversation

@liwilliam2021
Copy link
Contributor

@liwilliam2021 liwilliam2021 commented Jul 9, 2025 •
edited by ellipsis-dev bot
Loading

Added an option in auto-approve settings to automatically timeout long-running tool operations and suggest fallback options. The timeout threshold is 1 minute and can be configured. Suggested fallback options are generated by the model & are presented as a follow-up question.

This is helpful in preventing hanging.

The timeout option will remain hidden in settings until a few more PRs are merged, upon which it will be moved to the autoapprove menu.

Here is a feature loom: https://www.loom.com/share/882d05d3ae104dbb8b664ef64c0418ca

image

Here is the architecture diagram:
image
The TimeoutManager manages timeout operations generated by the ToolExecutionWrapper. The TimeoutFallbackHandler creates user-facing timeout responses by making a call to the AI generator. We have static suggestions as a fallback. We also added a set of time out tests.

In separate PRs, we will enable

  • More granularity with the timeout options
  • More sophistication around command timeouts
  • The ability for the model to override the timeout

Important

Introduces a configurable timeout feature for tool operations with automatic cancellation and fallback suggestions, including settings and UI updates, and adds tests for functionality.

  • Behavior:
    • Adds timeout feature for tool operations with a default of 1 minute, configurable via global-settings.ts.
    • Implements TimeoutManager and TimeoutFallbackHandler for managing and handling timeouts.
    • Adds tool_timeout event capture in TelemetryService.ts.
  • Settings:
    • Adds toolExecutionTimeoutMs and timeoutFallbackEnabled to settings in global-settings.ts and ClineProvider.ts.
    • Updates UI components in AutoApproveSettings.tsx and SettingsView.tsx to include timeout settings.
  • Tests:
    • Adds tests for timeout functionality in timeout-fallback.spec.ts, timeout-integration.spec.ts, and other test files.
  • Localization:
    • Updates localization files for new timeout-related strings in multiple languages.

This description was created by Ellipsis for 20a27f7. You can customize this summary. It will automatically update as commits are pushed.

@liwilliam2021 liwilliam2021 requested review from cte, jr and mrubens as code owners July 9, 2025 15:36
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. enhancement New feature or request labels Jul 9, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jul 9, 2025 •
edited
Loading

✅ No security or compliance issues detected. Reviewed everything up to 20a27f7.

Security Overview
  • 🔎 Scanned files: 62 changed file(s)
Detected Code Changes

The diff is too large to display a summary of code changes.

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 9, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 9, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 9, 2025
daniel-lxs
daniel-lxs reviewed Jul 11, 2025
View reviewed changes
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @liwilliam2021, I left a couple of suggestions that are worth taking a look at.

Let me know what you think!

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 11, 2025
@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 16, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 16, 2025
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Jul 16, 2025
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 16, 2025
@liwilliam2021 liwilliam2021 changed the title Tool timeout + recover from hanging feat: Tool timeout + recover from hanging Jul 17, 2025
daniel-lxs
daniel-lxs approved these changes Jul 17, 2025
View reviewed changes
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 17, 2025
@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Review] in Roo Code Roadmap Jul 17, 2025
mrubens
mrubens reviewed Jul 18, 2025
View reviewed changes
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice I still haven’t seen any tool hang other than execute_command and trying to call a MCP that doesn’t exist. I wonder if we should start with just the telemetry to get a sense of whether it’s worth it to add the rest of this functionality.

@liwilliam2021
Copy link
Contributor Author

liwilliam2021 commented Jul 18, 2025

In practice I still haven’t seen any tool hang other than execute_command and trying to call a MCP that doesn’t exist. I wonder if we should start with just the telemetry to get a sense of whether it’s worth it to add the rest of this functionality.

Hmm yeah that's not a bad idea-- I originally overengineered this as a starting point to build off better error handling for roomotes (AI fallbacks, maybe some timeouts from terminal snapshots) that I never got around to fully. I recall there were also some hangs w/ file reads if the context was messed up. If we think the current command execution fix is sufficient I'm happy to save ourselves all some time and drop this project entirely? I also don't want to overcrowd the repo with code we don't need.

Another thought it we want to keep building on this: I have another branch that has code to set custom timeout lengths for different tool executions. What if we took that and just hid the timeouts for every tool that's not "execute_command". This would in effect make this PR a settings option for the existing Roomotes command timeout functionality.

@daniel-lxs
Copy link
Member

daniel-lxs commented Jul 30, 2025

Closing for now, feel free to reopen it if needed

@daniel-lxs daniel-lxs closed this Jul 30, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrubens mrubens mrubens left review comments

@daniel-lxs daniel-lxs daniel-lxs approved these changes

@cte cte Awaiting requested review from cte cte is a code owner

@jr jr Awaiting requested review from jr jr is a code owner

Assignees

No one assigned

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Needs Review size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Roo Code Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@liwilliam2021 @daniel-lxs @mrubens @hannesrudolph